Spatially variable Robin BC bug fixes #445
Conversation
…warning if RobinBoundaryCondition is constructed with 0 stiffness and 0 damping
There was a problem hiding this comment.
Pull Request Overview
This PR refactors XML parameter naming for spatially variable Robin boundary conditions and adds validation warnings. It replaces the specific Robin_vtp_file_path element with the more general Spatial_values_file_path element to maintain consistency with other boundary condition types.
- Replace
<Robin_vtp_file_path>with<Spatial_values_file_path>in XML configuration - Add warning when Robin boundary condition has both stiffness and damping set to zero
- Update test cases to use the new XML format
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cases/ustruct/spatially_variable_robin/solver.xml | Updates XML element name for Robin BC file path |
| tests/cases/struct/spatially_variable_robin/solver.xml | Updates XML element name for Robin BC file path |
| Code/Source/solver/read_files.cpp | Changes parameter reference from robin_vtp_file_path to spatial_values_file_path |
| Code/Source/solver/RobinBoundaryCondition.h | Moves constructor from inline to declaration only |
| Code/Source/solver/RobinBoundaryCondition.cpp | Implements constructor with zero-value warning logic |
| Code/Source/solver/Parameters.h | Removes robin_vtp_file_path parameter declaration |
| Code/Source/solver/Parameters.cpp | Removes robin_vtp_file_path parameter initialization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| */ | ||
|
|
||
| #include "RobinBoundaryCondition.h" | ||
| #include <iostream> |
There was a problem hiding this comment.
Using std::cout directly is not recommended for library code. Consider using a proper logging framework or at minimum std::cerr for warnings to avoid mixing output streams.
| { | ||
| // Warning if both stiffness and damping are zero | ||
| if (uniform_stiffness == 0.0 && uniform_damping == 0.0) { | ||
| std::cout << "WARNING: Robin boundary condition has both stiffness and damping values set to zero. " |
There was a problem hiding this comment.
Warning messages should be written to std::cerr instead of std::cout to properly separate error/warning output from regular program output.
| std::cout << "WARNING: Robin boundary condition has both stiffness and damping values set to zero. " | |
| std::cerr << "WARNING: Robin boundary condition has both stiffness and damping values set to zero. " |
There was a problem hiding this comment.
There is no difference between the cout and cerr output message, just that cerr does not buffer data.
Warning messages should be written using simulation->logger so they will appear in the histor.dat file.
Should we throw when the both the stiffness and damping values are zero ?
aabrown100-git
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #445 +/- ##
==========================================
+ Coverage 67.42% 67.46% +0.03%
==========================================
Files 169 170 +1
Lines 34142 34146 +4
Branches 5727 5729 +2
==========================================
+ Hits 23021 23035 +14
+ Misses 10983 10972 -11
- Partials 138 139 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Current situation
Resolves #444
Release Notes
<Robin_vtp_file_path>xml element with<Spatial_values_file_path>for providing path to spatially variable Robin boundary condition .vtp file.RobinBoundaryConditionconstructor if bothstiffnessanddampingare 0.mesh_scale_factoris not 1.Testing
Modifies
struct/spatially_variable_robinandustruct/spatially_variable_robintests with new xml format.Code of Conduct & Contributing Guidelines